ref: eslint-plugin-boundaries #92031
Conversation
github-actions
bot
added
the
Scope: Frontend
| from: ['sentry*'], | ||
| allow: ['core*', 'sentry*'], |
There was a problem hiding this comment.
from and allow are micromatch patterns matched against the type defined in the above settings. I’ve “grouped” all sentry app related things as ”sentry-“ so that we can target them with ”sentry*” here.
So this means: sentry can import from sentry (itself) and from everything core, and since the default is disallow, everything else is forbidden and will yield a warning.
| type: 'core-button', | ||
| pattern: 'static/app/components/core/button', | ||
| }, | ||
| { | ||
| type: 'core', | ||
| pattern: 'static/app/components/core', | ||
| }, |
There was a problem hiding this comment.
note: we can remove core-button once we’ve isolated everything in core. core-button is a separate group now so that we get fewer violations. Since we match with core*, both groups are included.
TkDodo
changed the title
I think we need to scope that to only stories.tsx and .mdx files so that we don't end up with some component actually importing something from storybook 😅 |
yes, good points. I made
I’ll do the same thing for tests, just in case! |
Codecov ReportAll modified and coverable lines are covered by tests ✅ ✅ All tests successful. No failed tests found. Additional details and impacted files@@ Coverage Diff @@
## master #92031 +/- ##
==========================================
- Coverage 87.93% 87.62% -0.31%
==========================================
Files 10174 10365 +191
Lines 583373 587523 +4150
Branches 22596 22594 -2
==========================================
+ Hits 512966 514838 +1872
- Misses 69955 72264 +2309
+ Partials 452 421 -31 |
|
test boundaries were a bit harder to set-up, as our tests are all over the place, as are mocks, fixtures and test-utils. I really think we should consolidate this in a follow-up, my idea would be to have all tests and test utils co-located in sub-directories called anyways, with the current setup, we get the following remaining warnings, which I would try to tackle in a follow-up: |
This PR adds the
eslint-plugin-boundariesto define boundaries between our apps. In general, there are currently 3 boundaries we want to have:sentry: the sentry app, including sentry related code in/tests.gsAdmin: the getsentry app, including getsentry related code in/tests.gsApp: the admin appgenerally, the rules are:
sentrycannot import fromgsAdminorgsAppgsAppcan importsentry, but notgsAdmingsAdmincan import everythingFurther, we want to extract
sentry/app/components/coreto their own “boundary”, which means that ideally:core/componentscore/componentscan’t import anything fromsentry,gsAdminorgsApp.As a first proof of concept, we’ve added specific rules for
core/components/buttonto see what it takes to decouple it from the rest.There are currently 23 violations that would need to be resolved, which is why the rule is set to
warningonly.Once we are done with everything, this rule can and likely should replace the boundaries we have set via
no-restricted-imports, as the rule is better grouped and more powerful.